Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tables in config.extra can be merged with those in theme.extra #1100

Merged
merged 3 commits into from
Jul 29, 2020
Merged

Tables in config.extra can be merged with those in theme.extra #1100

merged 3 commits into from
Jul 29, 2020

Conversation

southerntofu
Copy link
Contributor

This is a bugfix ported from #997 which allows to use tables (subsections) in config.extra/theme.extra and have them merged without losing information.

Example config.toml:

[extra]
[extra.foo]
bar = "something"

Example theme.toml:

[extra]
[extra.foo]
bar = "default"
baz = "some more things"

Without this patch, accessing extra.foo.baz in a template will fail because the add_theme_extra function will entirely replace the theme's extra.foo with the config's extra.foo without merging child keys (bar, and baz in this example).

The behavior introduced here is to crash when the theme is expecting a table which is overridden by some data of another type in the config. Not sure if that's the best approach.

Keats
Keats reviewed Jul 27, 2020
View reviewed changes
components/config/src/config/mod.rs Outdated Show resolved Hide resolved
components/config/src/config/mod.rs Outdated Show resolved Hide resolved
@southerntofu
Copy link
Contributor Author

Recursion is here. Any further remarks?

@BertalanD
Copy link

I am sorry if it is not relevant, but I think the translations for the page's language should also be exposed in the extra object, to make transparent localization of templates easier.

@Keats
Copy link
Collaborator

Keats commented Jul 28, 2020

translations is going to be dumped in favour of fluent (#1040 don't mind that it is closed) so I wouldn't worry too much about it

@BertalanD
Copy link

BertalanD commented Jul 28, 2020 •
edited
Loading

I am currently working on that, will soon add a comment with my plans.

I think translations should be kept, because Fluent is too clunky for basic things like using the correct localized URL for a link. Many themes also rely on setting extra variables for things like page title, author name and copyright text, and these would all need to be rewritten to get these things from .ftl files, not to mention all of the user's config.

In my opinion, Fluent is best used only for creating internationalized templates/themes.

Will elaborate on that PR, but I am a noob.

@Keats Keats merged commit 7e7bf2b into getzola:next Jul 29, 2020
@Keats
Copy link
Collaborator

Keats commented Jul 29, 2020

@southerntofu do you want to become a member/admin of the Zola org? You're doing great work

@Keats
Copy link
Collaborator

Keats commented Jul 29, 2020

@BertalanD if we keep translations then we would have two ways of handling translations in the same app which is clearly not ideal.

@southerntofu
Copy link
Contributor Author

do you want to become a member/admin of the Zola org? You're doing great work

Thanks, i accepted the invitation (i'm honored). I'm not sure what that entails but i intend to continue contributing however i can. Is there a team meeting planned sometime soon so i can get up to speed? Or a IRC/XMPP chatroom i can join?

if we keep translations then we would have two ways of handling translations in the same app which is clearly not ideal.

I understand @BertalanD 's request for a simpler system for very straightforward cases. Maybe we could use lang keys in config/theme extra for this? Like extra.{en,fr}.dateFormat. If that sounds reasonable to you both, we could even automatically load the translations table into the extra table for backwards-compatibility.

@Keats
Copy link
Collaborator

Keats commented Jul 29, 2020

I'm not sure what that entails but i intend to continue contributing however i can. Is there a team meeting planned sometime soon so i can get up to speed? Or a IRC/XMPP chatroom i can join?

Mostly just planning the next versions via GitHub project and fixing bugs/docs, reviewing PRs. We can do an async meeting on https://zola.discourse.group/ so more people can collaborate.
The focus for 0.12 is the updated zola serve + bug fixes. I don't think it makes sense to get fluent in there since it needs a lot of testing/fine-tuning probably.
I don't think there are other big features coming though.

@southerntofu southerntofu mentioned this pull request Feb 26, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Keats Keats Keats left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@southerntofu @BertalanD @Keats